-
-
Notifications
You must be signed in to change notification settings - Fork 504
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix possible setting options list when camera uses exclusions #1548
Conversation
4848e58
to
a30687a
Compare
if (_current_settings[parameter.first].needs_updating) { | ||
continue; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed those lines because an exclusion does not depend on needs_updating
. I could verify in the test camera_definition that the unit test below was wrong: it was exposing CAM_VIDFMT
when it should actually have been excluded.
@@ -1633,14 +1633,15 @@ void CameraImpl::refresh_params() | |||
// any other possible settings to change. However, we still would | |||
// like to notify the current settings with this one change. | |||
notify_current_settings(); | |||
notify_possible_setting_options(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one line is the actual fix I needed 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Haha, ok!
Working with the MAVSDK-Swift sample app! |
Fedora CI issue is unrelated. |
Currently, parameters are refreshed when they are changed. If the camera definition defines an "update" dependency (not sure when that's done), then a parameter that is modified triggers a refresh in those update dependencies.
However, what is currently missing is a way to refresh parameters that were excluded. Let's have a look using the python
examples/camera_params.py
example.Without this PR, say the camera is in VIDEO mode, with exposure set to "Aperture Priority". The output will be:
Note how there is no shutter option, because it is excluded by the aperture option. Now say you move to "Manual Exposure", the shutter option will not appear. This PR aims at fixing that, such that switching between "Manual", "Program Auto", "Shutter Priority" and "Aperture Priority" update the list accordingly.
This issue was first discovered in the MAVSDK-Swift sample app. I will test it there before setting this PR as "ready".